Enhanced redis security: ACL and TLS #17577
Conversation
davidpil2002
requested review from
qiluo-msft,
xumia and
lguohan
as code owners
|
The feature was built and tested locally when including all the submodule modifications. For now, I would appreciate if you could review all the PRs related. |
liat-grozovik
changed the title
|
In this PR's description, all the code PR link text is "Added support for LDAP". Is it a mistake? |
| keyUsage = digitalSignature, keyEncipherment | ||
| nsCertType = client | ||
| _END_ | ||
|
|
There was a problem hiding this comment.
The CA cert need protect by file permission? for example only RW/Admin user can modify it?
There was a problem hiding this comment.
It's a nice addition, but currently we generated a cert just to have TLS working.
We tried performing the TLS without a certificate but did not manage to do so.
I think that adding some write protect (for admin only) is possible in this case.
@davidpil2002 - please review it :)
There was a problem hiding this comment.
yes - the file was copied to the filesystem with the owner admin,
the code can be seen in this PR in the filename
files/build_templates/sonic_debian_extension.j2
|
@liuh-80 Any additional comments on this PR and the related PRs? |
No for this PR, for related PRs I suggest:
|
yes - a mistake with the name not with the pointer linked, |
davidpil2002
force-pushed
the
dev-redis-acl-support
branch
from
87cae7e to
a8856f9
Compare
|
@Yarden-Z @davidpil2002 are you planning to handle the conflict and to have it ready for merge in 1 week or we should de prioritize it and have it for next release? if needed for 202405 we need you to make it ready and community to prioritize the review. |
We will not make it in time for this release, please move it to the next one. |
Why I did it
Enhance Redis Security.
Add Redis ACL support
Add Redis TLS support
Work item tracking
How I did it
Add Redis ACL support by creating 2 users nad configure them in Redis Server.
one admin user who has access to any commands and operations like write/read
one monitor user who has access to read-only
Add Redis TLS by enable it in Redis Server.
How to verify it
Good flow:
each flow that exists today that uses the Redis DB should continue to be supported
Bad flow:
Add a new user which not belong to the admin group, and try to write to the Redis DB, this operation should be blocked.
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)